Should this variable be documented?

yes, totally! I think it's worth discussing what is the best way to disable fancycompleter, if anybody has other ideas I'm happy to hear

documentation added in b710bce

Should this have a licence grant (e.g. BSD-3-Clause)? If so, we should add it here and to the included software licences section.

Another option, which might be slightly nicer, is for the contributors of the relevant bits of fancycompleter to sign the CLA and have this work relicenced under the PSF licence. This seems to include @blueyed, @theY4Kman, @singingwolfboy, and @slinkp.

I'm happy to do so

I only made this one obsolete commit to fancycompleter 13 years ago, but i'll sign if it helps :) How do I do that?

I'm not expert in licensing issue. I'm happy to do/sign whatever is needed to land the PR

The CLA can be signed here: https://cla.python.org/

How much time does this add to the autocompletion? I did a quick test with

>>> import sys
>>> sys.[TAB]

Then 86 names are processed with about 10 ms spend in colorize_matches alone. For modules or classes with more attributes the time would be longer. The processing does not seem needed since the response of the autocompletion is [ not unique].

Are there any benchmarks for the full autocompletion or rules-of-thumb how long it should take?

(maybe these questions are better suited for a separate issue or followup PR)

Are there any benchmarks for the full autocompletion or rules-of-thumb how long it should take?

I don't think there are any but I think that as long as is not slow enough even if the module is too big I don't think it matters. We can always also deactivate if there are too many elements in the module

Then 86 names are processed with about 10 ms spend in colorize_matches alone.

this seems way too much.
I ran this super simple benchmarks on my machine:

import sys
import time
from _pyrepl.fancycompleter import Completer

def gendict(n):
    return {
        f'item{i}': i
        for i in range(n)
    }

class MyClass:
    pass

def main():
    obj = MyClass()
    comp_col = Completer({'obj': obj}, use_colors=True)
    comp_nocol = Completer({'obj': obj}, use_colors=False)

    for i in range(0, 10000, 1000):
        obj.__dict__ = gendict(i)
        a = time.perf_counter()
        comp_col.attr_matches('obj.')
        b = time.perf_counter()
        comp_nocol.attr_matches('obj.')
        c = time.perf_counter()

        t_col = b - a
        t_nocol = c - b
        diff_pct = (t_col - t_nocol) / t_nocol * 100
        print(f"{i:5d}: col={t_col*1000:.1f}ms  nocol={t_nocol*1000:.1f}ms  diff={diff_pct:+.2f}%")

main()

and I got this results:

❯ ./python bench_fancycompleter.py 
    0: col=0.1ms  nocol=0.1ms  diff=+18.30%
 1000: col=0.7ms  nocol=0.7ms  diff=+5.81%
 2000: col=1.6ms  nocol=1.4ms  diff=+13.75%
 3000: col=2.4ms  nocol=2.0ms  diff=+21.95%
 4000: col=2.9ms  nocol=2.4ms  diff=+19.63%
 5000: col=3.8ms  nocol=5.0ms  diff=-23.60%
 6000: col=5.3ms  nocol=4.5ms  diff=+15.84%
 7000: col=5.0ms  nocol=4.6ms  diff=+9.01%
 8000: col=6.9ms  nocol=7.6ms  diff=-9.75%
 9000: col=7.0ms  nocol=6.9ms  diff=+1.47%

so, on my machine it spends 7ms to process 9000 names, and the difference between colors and no colors is basically noise.

The example minimal example above never reaches colorize_matches because there is a common prefix.
If I replace comp_col.attr_matches('obj.') with comp_col.attr_matches('obj.item') I get this output

    0: col=0.1ms  nocol=0.0ms  diff=+95.88%
 2000: col=85.6ms  nocol=1.0ms  diff=+8489.94%
 4000: col=166.8ms  nocol=1.9ms  diff=+8763.17%
 6000: col=286.6ms  nocol=3.6ms  diff=+7940.23%
 8000: col=330.9ms  nocol=4.5ms  diff=+7297.60%
10000: col=429.9ms  nocol=5.1ms  diff=+8394.79%
12000: col=509.7ms  nocol=6.1ms  diff=+8308.30%
14000: col=632.8ms  nocol=9.4ms  diff=+6627.23%
16000: col=1308.4ms  nocol=29.5ms  diff=+4331.26%
18000: col=2131.1ms  nocol=35.8ms  diff=+5855.55%

oh right, sorry for the silly mistake. Indeed, this is what I get on my machine:

❯ ./python bench_fancycompleter.py 
    0: col=0.1ms  nocol=0.1ms  diff=+16.43%
 1000: col=9.6ms  nocol=1.0ms  diff=+826.21%
 2000: col=17.8ms  nocol=1.3ms  diff=+1293.67%
 3000: col=26.6ms  nocol=1.8ms  diff=+1377.77%
 4000: col=33.1ms  nocol=2.3ms  diff=+1335.15%
 5000: col=40.8ms  nocol=3.7ms  diff=+996.66%
 6000: col=54.1ms  nocol=3.7ms  diff=+1368.12%
 7000: col=56.0ms  nocol=4.1ms  diff=+1253.30%
 8000: col=63.4ms  nocol=6.7ms  diff=+851.07%
 9000: col=71.9ms  nocol=6.0ms  diff=+1095.03%

I'm not an UI expert at all, but I think that anything <100ms is perceived as "instantaneous" by the user, so unless we have modules with >9000 we should be safe 😅.

Jokes apart, I see that on your machine things are slower, but I still think that with "normal" number of completions the delay won't be noticeable (and, anecdotally, I never noticed in all these years).

Anecdotally, the timings are from my fast machine, and I have noticed the new pyrepl being slower (not specifically this PR though). With the commit moving the gettheme() out of the loop performance is much better:

    0: col=0.1ms  nocol=0.0ms  diff=+114.87%
 1000: col=1.8ms  nocol=0.5ms  diff=+257.26%
 2000: col=3.2ms  nocol=1.1ms  diff=+187.30%
 3000: col=3.9ms  nocol=1.4ms  diff=+182.85%
 4000: col=4.9ms  nocol=2.0ms  diff=+143.63%
 5000: col=6.8ms  nocol=2.9ms  diff=+137.13%
 6000: col=8.0ms  nocol=3.2ms  diff=+152.49%
 7000: col=10.0ms  nocol=3.5ms  diff=+185.24%
 8000: col=10.9ms  nocol=4.3ms  diff=+153.73%
 9000: col=12.1ms  nocol=4.8ms  diff=+149.98%

So I am happy with the current performance of the PR!

Please keep this class in alphabetical order, like the others.

Same for the fancycompleter below.

done in 3a6bcd3

Could this be the following?

no, it doesn't work in case of e.g. a.b.c.<TAB>

I see. In general I like to avoid using eval in code (because of safety and performance reasons). Not using eval here would add a few lines of code though. Leaving the decision on this to a core dev

Should we color anyway for consistency?

no. When you return a single completion, readline inserts it in the prompt. If we return colors here, we would be ANSI codes in the prompt, which is surely not what you want

Agreed, changing the colors in the prompt is not what we want. Could you update the comment with the reason?

This does not sort the values. Maybe revert the commit that introduced this if it is too much trouble to fix this.

yes, pro tip for my future self: don't write code when you are sleepy :).
commit 7d2c790 reverts it, and also adds a comment to explain WHY we need to sort words.

The method does not seem to be used.

yes, it's used. We are subclassing rlcompleter.Completer and by overriding this method we prevent the insertion of '('.

If I remove this line, tests still pass. Why is this needed?

this is a valid question, and I admit I don't know the answer.
The code was originally introduced by this PR in fancycompleter, which merged some logic from CPython's rlcompleter.py:
pdbpp/fancycompleter#17

The current rlcompleter.py contains exactly the same lines:

So, I think it's a good idea to keep them in sync.

The behaviour in the rlcompleter was added 25 years ago in 4e20de5 and 46bd9a6 (for the __builtins__). I will check later whether the behaviour still makes sense, but changing that would be a different PR.

Can you use the same style as in rlcompleter.py though? E.g. use words.discard.

It was using words.discard, but then I committed this suggestion by @AA-Turner
#130473 (comment)

I'm happy with both 🤷‍♂️

If we can remove the line in fancycompleter and nothing breaks, we may as well do so.

Both the words.add('__class__') and words.discard("__builtins__") can be removed in rlcompleter.py without tests breaking (locally for me). The words.add('__class__') seems rather safe to me (if __class__ is an attribute of thisobject, it should be in dir(thisobject)).

Removing words.discard("__builtins__") is also fine with me, but I have no knowledge of the history here. The commit states

Do not expose builtins name as a completion; this is an implementation detail that confuses too many people. Based on discussion in python-dev.

The commit is so old, I could not find the python-dev archives containing the rationale. On modern Python there are a few more dunder names the might be confusing (e.g. in the global namespace we have __spec__, __package__, __build_class__ and a few more), so why would __builtins__ be special. Maybe in the python version back then __builtins__ was more prominent? Again, it is so long ago I cannot install a python version from that time on my system.

Note: if we decide to remove these, I would do it in a separate PR.

Why do we have to remove __builtins__? If I remove this part the unit tests still pass.

same answer as above, it's using the same logic as rlcompleter